-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Site Editor: update index view for pages #59950
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: -450 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
I think this highlights a couple things:
Should we try and "link" to the same "url" we use for "list view" in the other layouts too? I'm guessing that means illuminating the "details" panel entirely but to be honest, it's already very hidden with the current state of the PR. Should we wait for the unification of the details and inspector to advance more first? The other thing that this highlights is that our "frame" animation is kind of broken. The "right side" of the frame/preview should stay stable when you navigate from the root of the site editor into "pages". |
I don't think it needs to block this PR but noting that once merged we'll lose the cross-links from the Pages panel to relevant templates like 404 and search results. Related: #41716 It would be good to have some ideas in mind about how we replicate these in data views. Is it technically feasible to display these templates amongst regular pages? Or would a separate view ('Utility' or similar) within Pages work better? |
Be carefull, the usage of 14/15/16' laptop screens size is very frequent |
How challenging would it be to move the "views" functionality (left sidebar) into its own PR/experiment? |
Yes, definitely. I was planning to look into that as a follow-up given that it already works that way in
In the previous iteration we had them in the left sidebar, as they are now in
Can you expand on this? In the sidebar area, we have the default views (shipped in 6.5 and stable) plus the custom views (the only piece that remains hidden under the "wp-admin" experiment).
Yes. I'm landing the whole issue by pieces. |
It might be okay as a stop-gap, but I hope we can come up with something better. Fine to handle in a follow-up. |
@youknowriad I've fixed the back flow and it works like the existing pages in
Trunk Gravacao.do.ecra.2024-03-20.as.13.00.47.movThis PR Gravacao.do.ecra.2024-03-20.as.13.09.28.mov |
postId, | ||
canvas: 'edit', | ||
} } | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this change. This has multiple side effects:
- Adds a wrapper around the editor which probably have CSS impacts
- A link around a div is probably not valid HTML
- Most concerning for me: Causes a remount of the "preview/editor" component when you navigate to another route that doesn't have the link wrapper.
We already have an onClick
handler somewhere within the Editor
component, so it feels like it should be fixed there instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can look into exposing that onClick
handler or alternatives. This approach was what we had before the router refactoring, when the editor went back to the list view through the details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
445b150 exposes the editor's onClick
handle and uses it to set the URL params.
@oandregal In the last video you just shared, it's really unclear why the behavior is different when you "select item" or not. I feel like they should both do the same and go back to the list view no? |
I actually understood the opposite in your previous comment: that we should maintain the details view until the new experience that substitutes it is in place. This works the same it currently does in Gravacao.do.ecra.2024-03-20.as.16.12.29.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is a necessary evil before we complete the merge of "details" and "inspector".
I'll defer to @jameskoster and @SaxonF here.
This reverts commit 799c896.
445b150
to
10edde3
Compare
It was also strange for me to see how the back flow goes through details only if there's a previous selection ( |
I don't like that, I think the issue will fix itself when we remove the details step. |
I don't fully understand the technical implications but I agree with this. Ideally the W button should take you to list view. Is the concern that if we take you to list view then the details panel is effectively removed from the experience? For pages that doesn't seem like a huge deal because their detail panels offers very little value. But I suppose it would be a bit problematic for templates like Blog Home, where you can edit settings in the Details panel but not in the Inspector (yet). If we don't want to remove access to those settings then yes this is may be a necessary evil for now. |
This is how the list view works in I changed it to 1) make it similar to how table & grid work, which still use the details and 2) because when I asked around about this flow, people felt it was better to leave the details until the new thing lands (as I understand it, the details has a purpose that hasn't been yet absorbed by any other thing). If the feeling has changed, I'm happy to get rid of the details entirely, though it's a bit orthogonal to this work. Going to go ahead with this to unblock further work. |
Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: Marc-pi <mdxfr@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org>
Part of #59659
Related #59792 and #55083
What?
Makes the DataViews-powered Page the index view for the pages section.
Gravacao.do.ecra.2024-03-18.as.12.40.27.mov
Why?
It's the direction we want to go.
How?
Testing Instructions
Go to the Site Editor and visit the Pages section. It should display the list layout for pages.
Follow-ups
I've ran into the following issues while testing. This also happens in
trunk
so I plan to investigate separately:path=page
instead ofpostType=page
. The side-effect is that when coming back, it has thepath
and goes directly to list layout. It should go to the details view for the time being.